Refactor mass import page to use base list view instead of base edit view#6983
Refactor mass import page to use base list view instead of base edit view#6983solth merged 10 commits intokitodo:mainfrom
Conversation
|
also to check: #6883 |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
| Duplication | 0 |
TIP This summary will be updated as you push new changes. Give us feedback
@BartChris would you mind reviewing this pull request, with that particular issue in mind, as you worked on the mass import quite a lot yourself? |
|
Maybe @henning-gerhardt can also check if his issue with the role settings is fixed, because the error always comes down to the number of elements edited. |
|
I just noticed that the problems i encountered also appear in 3.9.0. |
|
As we at SLUB did not use the mass import feature and all changes are only focused on mass import I don't think that this pull request is fixing #6805 in first place. The comments in #6805 are switching fast from the original issue to an other maybe related issue and so I don't think that the main issue is fixed with this changes here. |
Yes, exactly. The linked issue was already fixed by #6810, I think, and the necessity to refactor the mass import form resulted from the changes in that pull request. |
|
@BartChris thanks a lot for your review and suggestions, which fixed the problems you reported. (I commited them manually instead of applying the suggestions via GitHub because some indentations seemed to be lost in the suggestions). I also increased the Would you mind checking the pull request again? |
3415193 to
1022eef
Compare
1022eef to
0c08d89
Compare
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
| Duplication | 2 |
TIP This summary will be updated as you push new changes. Give us feedback
Yes, that's suboptimal, and yes, we should address that in a separate pull request. |
|
@BartChris I get a |
d2ae1ff to
8ddb6f9
Compare
I do not get an error here (maybe you fixed that) but i also cannot edit and save the values (it keeps the old value). I would have to check if this ever worked on large tables. |
Yes, I was able to fix the problem with larger files by using the I tried updating the current row in but that did not work, either. |
Did this work in 3.8? I would say this is a Mass Import and values should be provided by the CSV in general, so i do not see it as a critical problem, if values cannot be edited directly in the UI. |
Yes, it did work in 3.8. And in 3.9 it works as well. In |
|
@BartChris as a temporary step I deactivated the option to edit datatable cells after uploading a CSV file, so that this feature is not offered in a broken state. I would suggest to go ahead and merge this pull request if you don't find any other problems with it, to at least restore the basic functionality of the mass import again. Once we get cell editing working again we can reactivate it afterwards in a separate pull request. What do you think? |
BartChris
left a comment
There was a problem hiding this comment.
Yes, i agree. The rest of the functionality works for me
We should probably make sure it isn't. Perhaps you could test with the same case in the I just noticed that the "Mass import" button still isn't activated immediately after uploading a CSV files that already contains all data (thus metadata of type |
|
I will check. Right now i suspect that this is again about external schema files i cannot resolve... |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
| Duplication | 2 |
TIP This summary will be updated as you push new changes. Give us feedback
|
Ok, after commenting out validationService.validateMappingFiles(importConfiguration.getMappingFiles());and deactivating validation here private String importProcessAndReturnParentID(String recordId, LinkedList<TempProcess> allProcesses,
ImportConfiguration importConfiguration, int projectID,
int templateID, boolean isParentInRecord, String parentIdMetadata,
boolean validateAgainstXmlSchema)it works. So this is not directly connected and about some validation issue. |
|
Same error on main, so definitely not connected! |
In Edit:It seems to be additionally checked in // validate external record against corresponding XML metadata schema(ta)
if (validateExternal && importConfiguration.getValidateExternalData()) {
validationService.validateExternalRecord(xmlContent, importConfiguration, identifier);
}But it seems like // validate mapping files against xslt schema
validationService.validateMappingFiles(importConfiguration.getMappingFiles()); is always called. In my case i suppose the mapping file validation let the process fail. |
Good that we found this out so it can be fixed as well! |
Interestingly, the setting in the import configuration actually seemed to work for me. When schema validation was activated in the configuration, I got the following result in mass import:
After deactivating the schema validation in the configuration, though, the import of the same IDs from the same catalog interface was succesful:
It seems the static parameter Maybe your import didn't really fail because of schema invalid metadata, but because the imported XML file or the import mapping XSLT did not contain valid XML to begin with? |
I have to check deeper, but my investigations right now seem to indicate that is not the validation of the catalog entries which fails for me, but the validation of the mapping files. And the latter might come down to the well known problem of external schema files, which are maybe blocked for me. kitodo-production/Kitodo/src/main/resources/schemata/xslt20.xsd Lines 57 to 64 in e1fdd90 Edit: OK, i get the same error in "normal" catalog import... will make an issue next week. |











Fixes #6805 by refactoring the mass import page so that it is not based upon the
baseEditView.xhtmltemplate anymore and thus can continue to use the requiredenctype="multipart/form-data"